Simplify the methodology for testing custom speedometer branches/commits
Categories
(Testing :: Raptor, task, P2)
Tracking
(firefox115 fixed)
Tracking | Status | |
---|---|---|
firefox115 | --- | fixed |
People
(Reporter: sparky, Assigned: sparky)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [fxp])
Attachments
(4 files)
This bug is for providing a simpler way to test custom speedometer branches, and revisions. At the moment, we need to make modifications to the test INIs which isn't very intuitive. We could provide some helper methods in mach-try-perf to do this.
Assignee | ||
Comment 1•2 years ago
|
||
:bgrins, could you elaborate on what is needed from a helper tool here? I'm thinking of implementing a flag in mach-try-perf that would modify the benchmark repo, branch, and revision used in the test, however, I'm not sure if that would resolve this request.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Comment 2•2 years ago
|
||
A flag that avoided touching speedometer-desktop.ini and speedometer-mobile.ini would indeed help. The basic workflow has been:
- Find the right repo / commit. This seems like it'd be trivial but I haven't found it to be in practice - take https://github.com/WebKit/Speedometer/pull/129 for example. Iif I click on "Commits" and scroll to the bottom I get to https://github.com/WebKit/Speedometer/pull/129/commits/c41429309d99afc947dbfb98c8c2c12b9c8fe46c which seems right, but failed in when I set that CI (sorry, I don't have an example push handy but in practice what i needed to do was go to the author's fork and find the last commit on the branch, or push the branch up to mozilla/speedometer and reference it from there).
- Make a commit changing it to the right value from (1) and push it to try. I've been using
./mach try perf --show-all --single-run --rebuild 5 --query "shippable \!32 speedometer3 'chrome | 'firefox | 'safari"
but i don't know if that's actually any good - it's just what I've landed on from trial & error - and so documenting the best syntax here would help. In particular I don't actually "want" to do a mozilla-central build for this usecase, I'd rather use the last nightly or something. - Make a separate commit and push (when necessary) for the base revision for the branch, as that won't necessarily / probably won't match the current revision in mozilla-central. This also requires some juggling around to find the right rev/repo.
- Pin a tab replacing the output of (2) and (3) to https://treeherder.mozilla.org/perfherder/compare?originalProject=try&newProject=try&newRevision=<NEW>&framework=13&originalRevision=<OLD>&page=1.
It requires a fair amount of juggling - largely from writing patches but also going around finding commit ids and keeping track of things. What would be nice for me is being able to reference just a repo+branch and have a tool figure out (2) (i.e. go find the latest rev on that branch rather than the user looking it up and copy/pasting and do the push). Figuring out (3) would be very nice as well but maybe that's too specific of a need to bake into the tool, and if so then documenting the appropriate git commands to find the base commit that can be pasted into the mach try perf
syntax would be a good improvement.
Updated•2 years ago
|
Assignee | ||
Comment 3•2 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #2)
A flag that avoided touching speedometer-desktop.ini and speedometer-mobile.ini would indeed help. The basic workflow has been:
- Find the right repo / commit. This seems like it'd be trivial but I haven't found it to be in practice - take https://github.com/WebKit/Speedometer/pull/129 for example. Iif I click on "Commits" and scroll to the bottom I get to https://github.com/WebKit/Speedometer/pull/129/commits/c41429309d99afc947dbfb98c8c2c12b9c8fe46c which seems right, but failed in when I set that CI (sorry, I don't have an example push handy but in practice what i needed to do was go to the author's fork and find the last commit on the branch, or push the branch up to mozilla/speedometer and reference it from there).
- Make a commit changing it to the right value from (1) and push it to try. I've been using
./mach try perf --show-all --single-run --rebuild 5 --query "shippable \!32 speedometer3 'chrome | 'firefox | 'safari"
but i don't know if that's actually any good - it's just what I've landed on from trial & error - and so documenting the best syntax here would help.
You should be able to use the Speedometer 3 category if you remove the --show-all
category - note that the --query argument will need to change to target the right categories: https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/tools/tryselect/selectors/perf.py#371-379
In particular I don't actually "want" to do a mozilla-central build for this usecase, I'd rather use the last nightly or something.
:bgrins, can you elaborate on this? I'm thinking you might be looking for the --artifact
option, but this sounds like it could be a bit different.
- Make a separate commit and push (when necessary) for the base revision for the branch, as that won't necessarily / probably won't match the current revision in mozilla-central. This also requires some juggling around to find the right rev/repo.
- Pin a tab replacing the output of (2) and (3) to https://treeherder.mozilla.org/perfherder/compare?originalProject=try&newProject=try&newRevision=<NEW>&framework=13&originalRevision=<OLD>&page=1.
It requires a fair amount of juggling - largely from writing patches but also going around finding commit ids and keeping track of things. What would be nice for me is being able to reference just a repo+branch and have a tool figure out (2) (i.e. go find the latest rev on that branch rather than the user looking it up and copy/pasting and do the push). Figuring out (3) would be very nice as well but maybe that's too specific of a need to bake into the tool, and if so then documenting the appropriate git commands to find the base commit that can be pasted into the
mach try perf
syntax would be a good improvement.
Thanks for all of this information! It's very helpful. What I think I'll do is implement a custom performance selector option in mach try perf
that will let us easily implement specialized, short-term task selectors which should be quite useful in the future too (e.g. --custom-perf-selector speedometer3 --speedometer3-args ...
). It'll have two methods for setting up your local m-c repo for a base revision, and a new revision try run. If either of them don't exist, we'd default to the current behaviour.
Comment 4•2 years ago
|
||
(In reply to Greg Mierzwinski [:sparky] from comment #3)
(In reply to Brian Grinstead [:bgrins] from comment #2)
A flag that avoided touching speedometer-desktop.ini and speedometer-mobile.ini would indeed help. The basic workflow has been:
- Find the right repo / commit. This seems like it'd be trivial but I haven't found it to be in practice - take https://github.com/WebKit/Speedometer/pull/129 for example. Iif I click on "Commits" and scroll to the bottom I get to https://github.com/WebKit/Speedometer/pull/129/commits/c41429309d99afc947dbfb98c8c2c12b9c8fe46c which seems right, but failed in when I set that CI (sorry, I don't have an example push handy but in practice what i needed to do was go to the author's fork and find the last commit on the branch, or push the branch up to mozilla/speedometer and reference it from there).
- Make a commit changing it to the right value from (1) and push it to try. I've been using
./mach try perf --show-all --single-run --rebuild 5 --query "shippable \!32 speedometer3 'chrome | 'firefox | 'safari"
but i don't know if that's actually any good - it's just what I've landed on from trial & error - and so documenting the best syntax here would help.You should be able to use the Speedometer 3 category if you remove the
--show-all
category - note that the --query argument will need to change to target the right categories: https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/tools/tryselect/selectors/perf.py#371-379In particular I don't actually "want" to do a mozilla-central build for this usecase, I'd rather use the last nightly or something.
:bgrins, can you elaborate on this? I'm thinking you might be looking for the
--artifact
option, but this sounds like it could be a bit different.
This is a special case where I'm wanting to test changes to the benchmark itself, with no other changes applied to the local build. In most cases we would be wanting to compare the same rev of the benchmark but with/without a local patch, whereas in this case I want to test different versions of the benchmark but with the same local codebase. I don't think we need to build a lot of infrastructure around this, since it's only interesting when benchmarks are actively being developed. If we can make the overall flow of pushing with a custom repo/branch/rev easier then that will also improve this use case.
Assignee | ||
Comment 5•2 years ago
|
||
Ok, thanks for the additional information! I'll implement something in mach try perf for this that'll be along the same lines of what I was thinking before. This is a subset of another use case we were thinking of which is doing arbitrary comparisons with mach try perf (e.g. multiple pref values).
Also, you might want to look into using the non-shippable linux/windows artifact builds (not available for mac) for this kind of testing. If you change the shippable
to !shipp !clang !rel
you'll be able to target them. I use them all the time for test-related changes to get my results quicker. That doesn't resolve the issue of comparing two arbitrary commits, but it could make your current workflow quicker. I'll file a bug to look into making it easier to target these.
Assignee | ||
Comment 6•2 years ago
|
||
This patch adds options for --benchmark-repository
, --benchmark-revision
, and --benchmark-branch
to make it easier to run custom benchark tests in Raptor. It also makes it possible to set the benchmark settings through mach try perf.
Updated•2 years ago
|
Assignee | ||
Comment 7•2 years ago
|
||
This patch adds the ability to run custom comparisons through "comparators". The code is modified to create a BasePerfComparator that provides the existing default behaviour. A new method, and command-line arguments are added to accept either a path to a custom comparator, or a name of a builtin comparator.
To add new builtin comparators, they must be added to the perfcomparators.py file with an @comparator
decorator. It must also subclass the BasePerfComparator. With this, a BenchmarkComparator is added that lets us pass custom options to raptor through the PERF_FLAGS environment variable (see --extra-args). To do this, some code needed to be moved around such as where the try_config is created. This lets us reset the configuration between the base, and new revision if needed.
The BenchmarkComparator lets us pass PR links as arguments, or the actual benchmark repo, revision, and branch. The PR link gets parsed into the required info using Github API requests. Note that links to direct commits do not work at the moment as the API doesn't provide the branch information for those kinds of requests.
Depends on D177030
Assignee | ||
Comment 8•2 years ago
|
||
:bgrins, I've made a couple patches to make this simpler for you. Could you test it out and see if it helps your workflow?
I made a BenchmarkComparator
that lets you pick a base-link, and new-link to compare in each of the pushes. You can try it out with the following command (you can add any additional settings you may need): ./mach try perf --comparator BenchmarkComparator --comparator-args base-link=https://github.com/WebKit/Speedometer/pull/149 new-link=https://github.com/WebKit/Speedometer/pull/149/commits/bac43087c4a98fabf28bd7aeb29d463371668bac
Here's a sample run for sp3 using the non-shippable linux artifact builds with the command above: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=afb7113244b8018d90eb0ea0d286eaa176a33475&newProject=try&newRevision=c596416d5e96b0fe5ada50a58ce37823316ea305
Note that I've only been able to get the PR link info parsed into the options. The direct commit links are harder to handle because the Github API doesn't return the branch they're found on (it looks like we'd need to do this locally). If you need to do comparisons with a non-PR commit, you'll need to find the repo/branch/revision information yourself, but you can pass them to mach try perf through the base-repo
, base-revision
, base-branch
(or with a new-
prefix for the second push). For example: ./mach try perf --comparator BenchmarkComparator --comparator-args base-repo=https://github.com/julienw/Speedometer base-revision=bac43087c4a98fabf28bd7aeb29d463371668bac base-branch=chartjs new-link=https://github.com/WebKit/Speedometer/pull/149/commits/bac43087c4a98fabf28bd7aeb29d463371668bac
Here's a sample run using the command above: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=eb68ab7e9e859fc3509c5cff7fded6d5ae73a045&newProject=try&newRevision=a90584e48d8e0ffbce7098195c1c7872c891ea07
It should also be easier for you to test things locally as Raptor will now have the --benchmark-repository
, --benchmark-revision
, and --benchmark-branch
options available. Note however that for updates to the test running in production branches, we'll still need to update the test INIs so that we have all the information about what we're running in CI within our PerfDocs: https://firefox-source-docs.mozilla.org/testing/perfdocs/raptor.html#speedometer3-b
Assignee | ||
Comment 9•2 years ago
|
||
This patch adds some unit tests for the comparators.
Depends on D177031
Assignee | ||
Comment 10•2 years ago
|
||
This patch adds documentation to the BasePerfComparator, and also adds some documentation for Comparators in general to the mach-try-perf Firefox source docs.
Depends on D177201
Comment 11•2 years ago
|
||
Comment 12•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/54c19401207a
https://hg.mozilla.org/mozilla-central/rev/dac6a95ef017
https://hg.mozilla.org/mozilla-central/rev/8f0fd75050ce
https://hg.mozilla.org/mozilla-central/rev/c315be87b0aa
Assignee | ||
Comment 13•2 years ago
|
||
:bgrins see comment 8 above for instructions on how to use the custom tooling.
Description
•